fix: land design-audit remediation with Production UI fixes#814
Conversation
…in-merge-safe-final-20260718-resubmit
Restore dashboard H1 expectations to Clinical Guide, mock service-detail registry records so 13YARN/City East load, drop forms assertions that conflicted with the live Refine/pathway UI, restore mode-menu click safety handlers, and correct the presentations redirect sitemap slug. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
…lback Suite-wide reducedMotion collapses CSS transition duration, so the phone dock duration assertion could not pass in Production UI. Prefer whitespace-empty presentations `query` falling through to `q`. Drop the Chromium no-preference override again now that the duration assert is gone. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Warning Review limit reached
Next review available in: 1 minute Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR updates RAG retrieval and query-correction infrastructure, adds database cleanup and indexing migrations, revises sitemap route classification, changes service/forms UI behavior, and refreshes navigation, schema, UI, and regression-test coverage. ChangesRoute inventory and repository tooling
RAG database hardening
RAG retrieval and abort wiring
Service and forms UI behavior
Navigation and page contracts
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/services/services-navigator-page.tsx (1)
297-309: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCompute checklist metrics from the selected services.
countssummarizes every search match, while “Review details” renders onlyselected. The checklist can therefore report criteria belonging to services the user did not select. Keep separate selected-service checklist counts and match-wide confidence counts.Proposed fix
- const counts = serviceNavigatorMetrics(matches); + const matchCounts = serviceNavigatorMetrics(matches); + const selectedCounts = serviceNavigatorMetrics(selected); ... - const confidenceTotal = counts.high + counts.medium + counts.low + counts.unknown; + const confidenceTotal = + matchCounts.high + matchCounts.medium + matchCounts.low + matchCounts.unknown; ... - ["Meets", counts.meets, CircleCheck, ...], + ["Meets", selectedCounts.meets, CircleCheck, ...],Use
matchCountsthroughout the source-confidence section.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/services/services-navigator-page.tsx` around lines 297 - 309, Update the metrics setup in the service navigator component to compute separate checklist counts from selected services while retaining match-wide counts for confidence metrics. Use the selected-service counts for the checklist rows and related checklist display logic, and use matchCounts throughout the source-confidence section; locate the affected logic around serviceNavigatorMetrics, the rows array, and confidenceTotal.src/lib/rag-candidate-sources.ts (1)
185-207: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftThread
signalthrough retrieval helpers.searchChunksWithTelemetryalready hasargs.signal, butsearchTextChunkCandidatesand the other candidate loaders drop it, so in-flight RPCs and table queries keep running after the request is aborted. Passsignalthrough these helper signatures, forward it intocallVersionedRetrievalRpc, and apply.abortSignal(signal)on direct query builders. Add a regression test that aborts while a query is pending.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/rag-candidate-sources.ts` around lines 185 - 207, Thread args.signal through searchTextChunkCandidates and all other candidate-loader helper signatures, forwarding it to callVersionedRetrievalRpc and applying abortSignal(signal) to direct query builders. Preserve existing retrieval behavior when no signal is provided, and add a regression test covering cancellation while a query is pending.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/services/services-navigator-page.tsx`:
- Line 608: The RightRail remount key should be removed so toggles such as
showConfidenceDetails persist across selection-count changes; in
src/components/services/services-navigator-page.tsx lines 607-608, keep
RightRail mounted and explicitly reset only invalid selection-dependent panel
state. In tests/audit-content-services-regressions.test.ts line 75, replace the
key-string assertion with regression coverage that verifies this behavior.
In `@supabase/migrations/20260714180000_patch_rag_and_corrector_scalability.sql`:
- Around line 63-78: Update the document title word trigger logic around the
DELETE/UPDATE cleanup and INSERT/UPDATE indexing conditions to use NULL-safe IS
DISTINCT FROM comparisons for status and title transitions. Preserve cleanup for
indexed-to-NULL changes and insertion for NULL-to-indexed or NULL-to-title
changes, then add regression coverage for each NULL transition.
In `@supabase/migrations/20260714190000_document_table_facts_trgm_idx.sql`:
- Around line 2-11: Replace the CREATE INDEX statement for
document_table_facts_text_trgm_idx with a no-op so fresh installations do not
build an index immediately removed by the later scalability migration. Preserve
forward cleanup for environments where the index already exists by updating the
later migration’s cleanup around document_table_facts_text_trgm_idx to use a
runner-compatible nonblocking operation, such as a concurrent drop, while
retaining the existing conditional behavior.
---
Outside diff comments:
In `@src/components/services/services-navigator-page.tsx`:
- Around line 297-309: Update the metrics setup in the service navigator
component to compute separate checklist counts from selected services while
retaining match-wide counts for confidence metrics. Use the selected-service
counts for the checklist rows and related checklist display logic, and use
matchCounts throughout the source-confidence section; locate the affected logic
around serviceNavigatorMetrics, the rows array, and confidenceTotal.
In `@src/lib/rag-candidate-sources.ts`:
- Around line 185-207: Thread args.signal through searchTextChunkCandidates and
all other candidate-loader helper signatures, forwarding it to
callVersionedRetrievalRpc and applying abortSignal(signal) to direct query
builders. Preserve existing retrieval behavior when no signal is provided, and
add a regression test covering cancellation while a query is pending.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b5b0fd54-6e60-4387-a615-6bfaec96cc65
📒 Files selected for processing (25)
docs/branch-review-ledger.mddocs/codebase-index.mddocs/site-map.mdscripts/check-github-action-pins.mjsscripts/generate-site-map.tssrc/app/differentials/presentations/route.tssrc/app/layout.tsxsrc/app/page.tsxsrc/components/forms/forms-home-page.tsxsrc/components/services/services-navigator-page.tsxsrc/lib/rag-candidate-sources.tssrc/lib/rag.tssupabase/drift-manifest.jsonsupabase/migrations/20260714180000_patch_rag_and_corrector_scalability.sqlsupabase/migrations/20260714190000_document_table_facts_trgm_idx.sqlsupabase/migrations/20260717010000_harden_rag_scalability_patch.sqlsupabase/schema.sqltests/audit-content-services-regressions.test.tstests/audit-navigation-auth-regressions.test.tstests/rag-variant-early-exit.test.tstests/site-map.test.tstests/supabase-schema.test.tstests/ui-smoke.spec.tstests/ui-tools.spec.tstsconfig.json
Keep the services RightRail mounted across selection-count changes, use IS DISTINCT FROM for title-word sync NULL transitions, and make the immediately-dropped table-facts trigram migration a no-op so fresh replays skip write-blocking create/drop churn. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/services/services-navigator-page.tsx`:
- Line 22: Move the invalid-selection panel reset currently performed in the
useEffect near the services navigator selection logic into the selection update
path. Update the handler or state transition that changes the selection to clear
the local panel state immediately when the new selection is invalid, then remove
the corresponding state update from useEffect while preserving valid-selection
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 00a0f19e-e1cb-42a4-b765-31a46ac7e0d2
📒 Files selected for processing (5)
src/components/services/services-navigator-page.tsxsupabase/migrations/20260714180000_patch_rag_and_corrector_scalability.sqlsupabase/migrations/20260714190000_document_table_facts_trgm_idx.sqltests/audit-content-services-regressions.test.tstests/supabase-schema.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- supabase/migrations/20260714180000_patch_rag_and_corrector_scalability.sql
- tests/audit-content-services-regressions.test.ts
- tests/supabase-schema.test.ts
Derived checklist/comparison expanded flags already hide invalid panels when selection size changes, so the effect that reset toggle state is unnecessary and trips react-hooks/set-state-in-effect. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
The remount key stays gone and panel validity is gated by derived expanded flags plus selection handlers, so drop the stale useEffect source expectations that broke Unit coverage. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Summary
mainwith Production UI fixes: Clinical Guide H1 contract, service-detail mocks, mode-menu blur/mousedownhandlers, and reduced-motion dock assert cleanup.contextOptionstyping, RAG/services navigator hardening, and schema/migration parity from the earlier design-audit merge attempts (chore: merge design-audit remediation to main #803/fix: finalize design audit regression hardening #806/fix: make Playwright reduced-motion config type-safe (policy clean) #807).Verification
Risk and rollout
Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
Summary by CodeRabbit